[ci] make MiKTeX downloads more reliable #3124

jameslamb · 2020-05-28T01:23:21Z

Moved this over from #3119 once we decided not to introduce Windows tests in that PR.

I learned in #3113 that the download link from the main MiKTeX site redirects to individual mirrors. I'm not sure what strategy it used but it seems to change pretty frequently. That makes me think the strategy is some very simple random load balancing that doesn't account for the health of the target mirror.

Here's an example from four requests executed within 15 seconds:

I think that pegging to a single mirror will reduce the frequency of errors like this downloading MiKTeX:

In theory that redirect mechanism they have is great because it means the download link should work whenever a particular mirror fails, but as we've seen it regularly routes us to a mirror that we can't successfully download from (for whatever reason).

Since the r-package jobs already rely on the University of Illinois mirror being up (to install additional LaTeX packages), I'd like to propose that we should just go directly to that mirror for MiKTeX too.

jameslamb · 2020-05-28T04:42:10Z

Ok this did fail on AppVeyor with an error related to MiKTeX but NOT one that is caused by (or could be solved by) the changes in this PR.

This PR occurs after the MIKTeX installer has been downloaded, when its being set up.

StrikerRUS

I'd prefer doing it via PS script as a general maintenance routine.

.ci/test_r_package_windows.ps1

jameslamb · 2020-05-29T00:36:32Z

Since this is only used by R builds, what is the benefit of re-writing it into Powershell? Web scraping in Powershell does not sound fun to me

StrikerRUS · 2020-05-29T02:16:10Z

Since this is only used by R builds, what is the benefit of re-writing it into Powershell?

As this particular script is needed only for R builds, it is OK from the first glance. But in general we should avoid using any specific languages for administration stuff. It will allow collaborators without knowledge of one specific programming language to maintain those tools in the future.

Going this way we may end up using dozens of languages calling one script from another for repo maintenance purposes and one ought to know all of them to just fix failing CI job.

Some other bad examples of unjustified use of Python:
https://github.com/microsoft/LightGBM/tree/master/helpers
https://github.com/microsoft/LightGBM/tree/master/.nuget

jameslamb · 2020-05-29T05:50:13Z

Since this is only used by R builds, what is the benefit of re-writing it into Powershell?

As this particular script is needed only for R builds, it is OK from the first glance. But in general we should avoid using any specific languages for administration stuff. It will allow collaborators without knowledge of one specific programming language to maintain those tools in the future.

Going this way we may end up using dozens of languages calling one script from another for repo maintenance purposes and one ought to know all of them to just fix failing CI job.

Some other bad examples of unjustified use of Python:
https://github.com/microsoft/LightGBM/tree/master/helpers
https://github.com/microsoft/LightGBM/tree/master/.nuget

Makes sense, thank you for the explanation!

In this case I really do think R code makes sense since this is specific to the R package, and since the task (scraping the source of an archive page to find the name of a file) is one that I personally don't know how to do in Powershell. I know it's possible, but it would take me a while and I'm guessing that what I come up with would be error prone.

We already have other R code in R-specific CI administration (https://github.com/microsoft/LightGBM/blob/master/.ci/lint_r_code.R), and I know that any time r-package tasks fail in CI, I'll get @-ed.

StrikerRUS · 2020-05-29T18:34:26Z

We already have other R code in R-specific CI administration

That script uses API of lintr package, so it is required to be written in R.

and I know that any time r-package tasks fail in CI, I'll get @-ed.

Yeah, this is exactly what I'm saying about! And it is not good, because maintainers can leave projects for some reasdon and it is a natural process. It will result in a some kind of "bus factor" (e.g. similar problems: dmlc/xgboost#4958).

Please treat all my thoughts above as a my general opinion to the whole repo management. I'm not against that this particular PR will add parsing a web page code in R.

jameslamb · 2020-05-30T16:01:28Z

I'm not against that this particular PR will add parsing a web page code in R.

Thanks for this @StrikerRUS . I decided to take learning how to do this in Powershell as a learning opportunity, and I'm really glad that I did! It was a lot easier than I expected. See the changes in 23cf95a

It was easier than I expected because I learned something amazing...you can run Powershell on a Mac!

This made it really easy to test and experiment in my usual dev environment 😀

jameslamb · 2020-05-30T16:02:16Z

.ci/test_r_package_windows.ps1

-Write-Output "Installing dependencies"
-$packages = "c('data.table', 'jsonlite', 'Matrix', 'processx', 'R6', 'testthat'), dependencies = c('Imports', 'Depends', 'LinkingTo')"
-Rscript --vanilla -e "options(install.packages.check.source = 'no'); install.packages($packages, repos = '$env:CRAN_MIRROR', type = 'binary', lib = '$env:R_LIB_PATH')" ; Check-Output $?
-


I'll undo this change. This doesn't have to move now that we aren't installing {httr} for the MiKTeX download!

undid these changes in 10e32fa, so now all that is left is the MiKTeX stuff.

If it build successfully I'll merge this! Hopefully it will help reduce frequency failing R builds that affect other PRs

StrikerRUS · 2020-05-31T21:18:27Z

@jameslamb

Thanks a lot for rewriting this in PowerShell!

It was easier than I expected because I learned something amazing...you can run Powershell on a Mac!

Yeah! PS Core is a great thing along with .NET Core 😃 ,

StrikerRUS

Thanks for the clean solution!

.ci/test_r_package_windows.ps1

Co-authored-by: Nikita Titov <[email protected]>

jameslamb · 2020-06-02T01:44:17Z

I just rebased to master to get the latest changes, especially #3133

Happy to see that GitHub Actions got triggered on this PR!

Since it isn't required yet (#3119 (comment)), I'll be sure to check them manually before merging.

* [ci] make MiKTeX downloads more reliable * switch to Powershell * remove extra slash * fixing stuff * revert install change * Update .ci/test_r_package_windows.ps1 Co-authored-by: Nikita Titov <[email protected]> Co-authored-by: Nikita Titov <[email protected]>

github-actions · 2023-08-24T12:13:26Z

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.

jameslamb added the maintenance label May 28, 2020

jameslamb requested a review from StrikerRUS May 28, 2020 01:23

jameslamb requested a review from Laurae2 as a code owner May 28, 2020 01:23

StrikerRUS reviewed May 28, 2020

View reviewed changes

.ci/test_r_package_windows.ps1 Outdated Show resolved Hide resolved

.ci/test_r_package_windows.ps1 Outdated Show resolved Hide resolved

jameslamb commented May 30, 2020

View reviewed changes

StrikerRUS approved these changes May 31, 2020

View reviewed changes

.ci/test_r_package_windows.ps1 Outdated Show resolved Hide resolved

.ci/test_r_package_windows.ps1 Show resolved Hide resolved

jameslamb and others added 6 commits June 1, 2020 20:42

[ci] make MiKTeX downloads more reliable

227b8c5

switch to Powershell

5a939f9

remove extra slash

eb8ad63

fixing stuff

b64e3a1

revert install change

cab58f0

Update .ci/test_r_package_windows.ps1

cf93d50

Co-authored-by: Nikita Titov <[email protected]>

jameslamb force-pushed the ci/download-miktex branch from a01dfd6 to cf93d50 Compare June 2, 2020 01:42

jameslamb merged commit 075513f into microsoft:master Jun 2, 2020

jameslamb deleted the ci/download-miktex branch September 7, 2020 05:26

github-actions bot locked as resolved and limited conversation to collaborators Aug 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ci] make MiKTeX downloads more reliable #3124

[ci] make MiKTeX downloads more reliable #3124

jameslamb commented May 28, 2020

jameslamb commented May 28, 2020

StrikerRUS left a comment

jameslamb commented May 29, 2020

StrikerRUS commented May 29, 2020

jameslamb commented May 29, 2020

StrikerRUS commented May 29, 2020 •

edited

Loading

jameslamb commented May 30, 2020

jameslamb May 30, 2020 •

edited

Loading

jameslamb May 31, 2020

StrikerRUS commented May 31, 2020

StrikerRUS left a comment

jameslamb commented Jun 2, 2020

github-actions bot commented Aug 24, 2023

[ci] make MiKTeX downloads more reliable #3124

[ci] make MiKTeX downloads more reliable #3124

Conversation

jameslamb commented May 28, 2020

jameslamb commented May 28, 2020

StrikerRUS left a comment

Choose a reason for hiding this comment

jameslamb commented May 29, 2020

StrikerRUS commented May 29, 2020

jameslamb commented May 29, 2020

StrikerRUS commented May 29, 2020 • edited Loading

jameslamb commented May 30, 2020

jameslamb May 30, 2020 • edited Loading

Choose a reason for hiding this comment

jameslamb May 31, 2020

Choose a reason for hiding this comment

StrikerRUS commented May 31, 2020

StrikerRUS left a comment

Choose a reason for hiding this comment

jameslamb commented Jun 2, 2020

github-actions bot commented Aug 24, 2023

StrikerRUS commented May 29, 2020 •

edited

Loading

jameslamb May 30, 2020 •

edited

Loading